Skip to content

Store exceptions in request env#994

Merged
lgebhardt merged 1 commit intoJSONAPI-Resources:masterfrom
travisofthenorth:store-exceptions
Mar 21, 2017
Merged

Store exceptions in request env#994
lgebhardt merged 1 commit intoJSONAPI-Resources:masterfrom
travisofthenorth:store-exceptions

Conversation

@travisofthenorth
Copy link
Copy Markdown
Contributor

@travisofthenorth travisofthenorth commented Mar 1, 2017

I know that there is an exception whitelist, and that all exceptions can be whitelisted in the latest version with the whitelist_all_exceptions configuration. IMO, this is tangential issue, because as a developer, I might want the gem to handle an exception for me as far as responding to users with error messages, but I also want this exception to be logged separately.

For example, I rely on a Rollbar middleware to report exceptions for alerting/monitoring of my services. Because the gem was swallowing exceptions (in this case a database timeout, which should absolutely sound the alarms), I wasn't aware of a critical app error. Again, I still want the gem to handle responding to users with error messages, but I want those server errors to be processed by the rest of my middleware chain.

@travisofthenorth
Copy link
Copy Markdown
Contributor Author

For reference, this is the Rollbar middleware I have in mind: https://github.com/rollbar/rollbar-gem/blob/master/lib/rollbar/middleware/rails/rollbar.rb#L26-L28

This is also what the ActionDispatch middleware sets, hence why Rollbar looks for it: https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/show_exceptions.rb#L46

@HoyaBoya
Copy link
Copy Markdown

I think this is a great PR. 👍

@lgebhardt lgebhardt merged commit 03f4937 into JSONAPI-Resources:master Mar 21, 2017
@lgebhardt
Copy link
Copy Markdown
Contributor

@travisofthenorth Thanks!

@travisofthenorth travisofthenorth deleted the store-exceptions branch March 21, 2017 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants